Support alpha in hexadecimal colors #3052
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vmcj
force-pushed
the
hex_shorthand_alpha
branch
from
d0b078e to
83e8167
Compare
meisterT
reviewed
Aug 7, 2025
webapp/src/Utils/Utils.php
Outdated
| */ | ||
| public static function parseHexColor(string $hex): array | ||
| { | ||
| // Ignore alpha in hexstrings |
There was a problem hiding this comment.
Why silently ignore the alpha channel?
There was a problem hiding this comment.
It might indeed a bit harsh to do this silently. Do we ever think we need the alpha channel? Otherwise I think I'll add a optional boolean for this behaviour (optionally also returning rgba instead of rgb), alternative is to check all calling functions and handle this there.
There was a problem hiding this comment.
It's unlikely that we need it. But I'm either for supporting it properly or not allowing it at all to avoid user confusion.
There was a problem hiding this comment.
The spec allows for sending such a string "#ff00ffaa" so we need to store it. I think my fix already makes that we never pick it up but I'll think about this some more. Maybe just supporting it is easier in that case.
vmcj
force-pushed
the
hex_shorthand_alpha
branch
8 times, most recently
from
5369d0c to
f4e085c
Compare
meisterT
approved these changes
Sep 15, 2025
No functional change intended.
vmcj
force-pushed
the
hex_shorthand_alpha
branch
from
1386766 to
d4e80ec
Compare
And add some more testing for those to also check for DOMjudge#123 format. The functions using the strings have moved from the TwigExtension to utils as the they don't really depend on Twig itself. This makes testing easier and can use this in the future for other elements. Co-authored-by: Tobias Werth <spam-by-github@never-mind.eu>
The contest validation breaks when the contestproblem has an invalid color PC^2 doesn't check the colors as they don't need them so mistakes in the problemset.yaml would also be fed to the feed.
This reverts commit b0b2b6e2ca30ea04d340f04ceb7a9eaee40b60f0.
This reverts commit 0353b080d18cfe7aeeefea96aad42d2247376ea0.
The Alpha is allowed by the CLICS spec but we don't really use those in the UI normally. The only place where we have to fully ignore the alpha is for the luminance. In that case we now by default merge it with a white background most likely this will be the same for either background but this was not tested. As bonus we now also test for the shorthand (#ABC) versuse (#AABBCC). The functions using the strings have moved from the TwigExtension to utils as the they don't really depend on Twig itself. This makes testing easier and can use this in the future for other elements. Co-authored-by: Tobias Werth <spam-by-github@never-mind.eu>
This reverts commit f231b96.
This reverts commit 9afb95b.
vmcj
force-pushed
the
hex_shorthand_alpha
branch
from
d4e80ec to
c5bc5f1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is as a basis to detect problematic colors coming from the primary.
I can't think of any place where any consumer would ever want to display the colors with some opacity. So I wonder if it's intentional that the spec links to a document which has this.